-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: update typevar handling when default is not set #7719
Conversation
I believe the behaviour change in #7606 is raising false positives when the `default` argument is supported but not specified. Ref: https://peps.python.org/pep-0696/#implementation
please review |
if default is None: | ||
default = not_set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the default not be None
? How does this differentiate default=None
from no default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the PEP:
- when default is unset,
__default__
would beNone
- if
default=None
,__default__
would beNoneType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get rid of our internal unset
thing then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yes - let me try and see if that works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
Change Summary
I believe the behaviour change in #7606 is raising false positives when the
default
argument is supported but not specified.Ref: https://peps.python.org/pep-0696/#implementation
Related issue number
#7606
Checklist
Selected Reviewer: @Kludex